Support logging into a registry if necessary #787

HarryMichal · 2021-06-07T19:54:49Z

This introduces handling of registries with private repositories/images. Practically speaking, if a used image lives in a registry behind a login wall, Toolbox will be aware of the fact and asks the user if they want to log in. This can be automated by combining the existing global --assumeyes option with newly introduced options --authfile and --creds in command create.

The authentication is done by Podman and Toolbox only serves as a mediator between the user and Podman. It does not record/store user's credentials in any way.

Examples of the new behaviour:

# Try to create from an unknown image
$ toolbox create -i registry.fedoraproject.org/no-image
Image required to create toolbox container.
Download registry.fedoraproject.org/no-image (500MB)? [y/N]: y
Error: The requested image does not exist
Make sure the image URI is correct.

# Try to create from an image in registry with authentication
$ toolbox -y create -i registry.redhat.io/image
Image required to create toolbox container.
Download registry.redhat.io/image (500MB)? [y/N]: y
The registry requires logging in.
Do you want to log into the registry and try to pull the image again? [y/N]: y
Username: <username>
Password:
Retrying to pull image registry.redhat.io/image
Created container: image-toolbox
Enter with: toolbox enter image-toolbox

# Create from an image in registry with authentication by providing login credentials
$ toolbox -y create -i registry.redhat.io/image --creds <username>:<password>
Could not pull image registry.redhat.io/image
The registry requires logging in.
Credentials were provided. Trying to log into registry.redhat.io
Login Succeeded!
Retrying to pull image registry.redhat.io/image
Created container: image-toolbox
Enter with: toolbox enter image-toolbox

Depends on #786, #823

Closes #754

containers#787

softwarefactory-project-zuul · 2021-06-07T20:08:02Z

Build succeeded.

unit-test : SUCCESS in 2m 20s
system-test-fedora-33 : SUCCESS in 6m 46s
system-test-fedora-34 : SUCCESS in 6m 35s
system-test-fedora-rawhide : SUCCESS in 11m 02s

Some registries contain private repositories of images and require the user to log in first to gain access. With this Toolbox tries to recognize errors when pulling images and offer the user the means to log in. The parsing process is quite fragile since Podman does not provide clear means to distinguish such cases and provided messages differ between registries. This should distinguish authorization errors with registry.redhat.io and docker.io. Testing of this feature is tricky as it relies on network which is inherently flaky. So, not everything added/changed by this will be tested. In the future we'll have to start mocking servers to be able to do so. containers#787

softwarefactory-project-zuul · 2021-06-08T09:06:04Z

Build failed.

unit-test : NODE_FAILURE in 0s
system-test-fedora-33 : NODE_FAILURE in 0s
system-test-fedora-34 : NODE_FAILURE in 0s
system-test-fedora-rawhide : NODE_FAILURE in 0s

Some registries contain private repositories of images and require the user to log in first to gain access. With this Toolbox tries to recognize errors when pulling images and offer the user the means to log in. The parsing process is quite fragile since Podman does not provide clear means to distinguish such cases and provided messages differ between registries. This should distinguish authorization errors with registry.redhat.io and docker.io. Testing of this feature is tricky as it relies on network which is inherently flaky. So, not everything added/changed by this will be tested. In the future we'll have to start mocking servers to be able to do so. containers#787

softwarefactory-project-zuul · 2021-06-08T09:18:10Z

Build failed.

unit-test : NODE_FAILURE in 0s
system-test-fedora-33 : NODE_FAILURE in 0s
system-test-fedora-34 : NODE_FAILURE in 0s
system-test-fedora-rawhide : NODE_FAILURE in 0s

HarryMichal · 2021-06-08T09:48:34Z

recheck

softwarefactory-project-zuul · 2021-06-08T10:03:49Z

Build failed.

unit-test : SUCCESS in 2m 34s
system-test-fedora-33 : FAILURE in 8m 11s
system-test-fedora-34 : FAILURE in 9m 06s
system-test-fedora-rawhide : FAILURE in 13m 10s

Some registries contain private repositories of images and require the user to log in first to gain access. With this Toolbox tries to recognize errors when pulling images and offer the user the means to log in. The parsing process is quite fragile since Podman does not provide clear means to distinguish such cases and provided messages differ between registries. This should distinguish authorization errors with registry.redhat.io and docker.io. Testing of this feature is tricky as it relies on network which is inherently flaky. So, not everything added/changed by this will be tested. In the future we'll have to start mocking servers to be able to do so. containers#787

softwarefactory-project-zuul · 2021-06-09T20:24:35Z

Build succeeded.

unit-test : SUCCESS in 2m 17s
system-test-fedora-33 : SUCCESS in 10m 06s
system-test-fedora-34 : SUCCESS in 9m 42s
system-test-fedora-rawhide : SUCCESS in 12m 42s

HarryMichal · 2021-06-09T21:45:20Z

I may have gotten an idea on how to test this reliably. Until I have it figured out or I give up, let's postpone merging this.

EDIT: I also found more info regarding the parsed error messages. I started to make more sense of them :). All thanks to reading the documentation \o/.

tpopela · 2021-06-10T04:23:36Z

src/pkg/podman/podman.go

+	return nil
+}
+
+// Pull pulls an image. Wraps around command 'podman pull'.


Extra pulls

I think that is intentional since Go tooling requires function comments to start with the name of the function. But I'd reword the function anyway :).

Some registries contain private repositories of images and require the user to log in first to gain access. With this Toolbox tries to recognize errors when pulling images and offer the user the means to log in or tell the user that the requested image/manifest does not exist. The parsing process relies on error messages that match the OCI specification[0]. Testing of this feature is tricky as it relies on network which is inherently flaky. So, as part of the setup two local image registries are created and these features can be tested against them. To prevent the removal of the registries during testing a flag --namespace is used in Podman during their creation. [0] https://github.com/opencontainers/distribution-spec/blob/main/spec.md#error-codes containers#787

softwarefactory-project-zuul · 2021-07-04T20:51:03Z

Build failed.

unit-test : SUCCESS in 3m 28s
system-test-fedora-33 : FAILURE in 11m 26s
system-test-fedora-34 : FAILURE in 12m 11s
system-test-fedora-rawhide : FAILURE in 17m 56s

containers#787

A subsequent commit will use this when retrying the 'podman pull' after logging the user into the registry for images that require authorization. containers#787

Some registries contain private repositories of images and require the user to log in first to gain access. With this Toolbox tries to recognize errors when pulling images and offers the user the means to log in. Some changes by Debarshi Ray. containers#787

Testing of this feature is tricky as it relies on network which is inherently flaky. So, as part of the setup two local image registries are created and these features can be tested against them. To prevent the removal of the registries during testing a different root for Podman is used. The only tested registry implementation is Docker[0]. [0] https://hub.docker.com/_/registry/ containers#787

The new additions to the system tests makes them take longer to run, and was causing them to timeout. containers#787

debarshiray

A couple of things about the test suite:

(You did a really good job setting up the registries as part of the test suite! Well done.)

debarshiray · 2021-07-04T22:36:32Z

test/system/000-setup.bats

 load 'libs/helpers'

 @test "test suite: Setup" {
    # Cache the default image for the system
    _pull_and_cache_distro_image $(get_system_id) $(get_system_version) || die
    # Cache all images that will be needed during the tests
    _pull_and_cache_distro_image fedora 32 || die
+    _pull_and_cache_distro_image rhel 8.4 || die


Can't we use a Fedora (or anything that's not UBI) image for testing?

debarshiray · 2021-07-04T22:37:56Z

test/system/000-setup.bats

+    $PODMAN --root "$PODMAN_REG_ROOT" logs docker-registry-auth
+
+    # Add UBI8 to created registries
+    run $SKOPEO copy "dir:${IMAGE_CACHE_DIR}/fedora-toolbox-32" "docker://localhost:50000/fedora-toolbox:32"


Typo in the comment? Is the comment really necessary? We didn't use one for the similar statement below. :)

softwarefactory-project-zuul · 2021-07-04T22:49:15Z

Build failed.

unit-test : SUCCESS in 2m 15s
system-test-fedora-33 : FAILURE in 10m 50s
system-test-fedora-34 : FAILURE in 10m 24s
system-test-fedora-rawhide : FAILURE in 17m 16s

This is meant to get a better understanding of a failed 'podman pull' invocation to understand whether pulling an image requires logging into the registry or not. Currently, 'podman pull' doesn't have a dedicated exit code to denote authorization errors, so this is meant to be a temporary workaround for that. Parsing the error stream is inherently fragile and tricky because there's no guarantee that the structure of the messages won't change, and there's no clear definition of how the messages are laid out. Therefore, this approach can't be treated as a generic solution for getting detailed information about failed Podman invocations. The error stream is used not only for dumping error messages, but also for showing progress bars. Therefore, all lines are skipped until one that starts with "Error: " is found. This is a heuristic based on how Go programs written using the Cobra [1] library tend to report errors. All subsequent lines are taken together and split around the ": " sub-string, on the assumption that the ": " sub-string is used when a new error message is prefixed to an inner error. Each sub-string created from the split is treated as a potential member of the chain of errors reported within Podman. Some real world examples of the 'podman pull' error stream in the case of authorization errors are: * With Docker Hub (https://hub.docker.com/): Trying to pull docker.io/library/foobar:latest... Error: Error initializing source docker://foobar:latest: Error reading manifest latest in docker.io/library/foobar: errors: denied: requested access to the resource is denied unauthorized: authentication required * With registry.redhat.io: Trying to pull registry.redhat.io/foobar:latest... Error: Error initializing source docker://registry.redhat.io/foobar:latest: unable to retrieve auth token: invalid username/password: unauthorized: Please login to the Red Hat Registry using your Customer Portal credentials. Further instructions can be found here: https://access.redhat.com/RegistryAuthentication [1] https://github.com/spf13/cobra/ https://pkg.go.dev/github.com/spf13/cobra containers#689 containers#786 containers#787

This way the standard error stream of the spawned binaries can be inspected to get a better understanding of the failure, while still being shown to the user when run with the '--verbose' flag. Unfortunately, this breaks the progress bar in 'podman pull' because the standard error stream is no longer connected to a file descriptor that's a terminal device. containers#689 containers#787 containers#823

containers#689 containers#787

A subsequent commit will use this when retrying the 'podman pull' after logging the user into the registry for images that require authorization. containers#689 containers#787

Some registries contain private repositories of images and require the user to log in first to gain access. With this Toolbox tries to recognize errors when pulling images and offers the user the means to log in. Some changes by Debarshi Ray. containers#689 containers#787

Testing of this feature is tricky as it relies on network which is inherently flaky. So, as part of the setup two local image registries are created and these features can be tested against them. To prevent the removal of the registries during testing a different root for Podman is used. The only tested registry implementation is Docker[0]. [0] https://hub.docker.com/_/registry/ containers#689 containers#787

The new additions to the system tests makes them take longer to run, and was causing them to timeout. containers#689 containers#787

softwarefactory-project-zuul · 2021-07-04T23:31:23Z

Build failed.

unit-test : SUCCESS in 2m 28s
system-test-fedora-33 : FAILURE in 10m 04s
system-test-fedora-34 : FAILURE in 11m 04s
system-test-fedora-rawhide : FAILURE in 16m 21s

debarshiray · 2021-07-05T00:00:47Z

Unless podman pull grows a dedicated exit code to denote authorization errors, we can't merge this upstream. However, downstream distributors who want this feature can use this patch-set at their own risk.

In the absence of a dedicated exit code from podman pull, this patch-set tries to parse the standard error stream of podman pull to guess whether the image requires logging into the registry or not. This sort of screen scraping is very fragile and is a bad idea.

First, the messages that get dumped in the error stream don't have any fixed structure or format, and can change from one Podman version to another. Moreover, the error stream is also used to show things like progress bars, so those need to be skipped when parsing it. This makes it very fragile to test whether the parsing works as expected across several Podman versions in several operating systems.

Second, since the error stream is also used to show progress bars for podman pull, redirecting it away from a terminal device (to intercept its contents) prevents the progress bar from being shown. We show the progress bar when toolbox --verbose ... is used because they are useful for checking the status of the pulls. Ultimately, we are downloading hundreds of megabytes as part of a critical operation, so having access to a working progress bar is important.

So, long story short, we either need podman pull to have a dedicated exit code for authorization failures, or we need to re-implement podman pull directly inside Toolbox so that we have better access to the errors.

HarryMichal · 2021-07-15T16:14:18Z

I'll close this since this approach is not acceptable long-term-wise. If the work is attempted again, it can be done in a separate PR.

HarryMichal added 3. Enhancement Improvement to an existing feature 6. Major Change May cause breakage 2. Host Realm The issue is related to what happens on the host machine where Toolbox is executed 2. Under The Hood Multiple areas of the app are influenced by this ticket labels Jun 7, 2021

HarryMichal added a commit to HarryMichal/toolbox that referenced this pull request Jun 7, 2021

[WIP] cmd/create, pkg/podman: Add capabilities for logging to registries

fc636da

containers#787

HarryMichal force-pushed the cmd/create/support-registry-authentication branch from 1155a56 to fc636da Compare June 7, 2021 19:55

HarryMichal changed the title ~~cmd/create, pkg/podman: Add capabilities for logging to registries~~ cmd/create, pkg/podman: Add capabilities for loging to registries Jun 7, 2021

HarryMichal force-pushed the cmd/create/support-registry-authentication branch from fc636da to 6808d35 Compare June 8, 2021 09:05

HarryMichal force-pushed the cmd/create/support-registry-authentication branch from 6808d35 to bbcd5e1 Compare June 8, 2021 09:17

HarryMichal marked this pull request as ready for review June 9, 2021 11:32

HarryMichal force-pushed the cmd/create/support-registry-authentication branch from bbcd5e1 to 4ab6577 Compare June 9, 2021 20:09

HarryMichal requested a review from debarshiray June 9, 2021 20:58

tpopela reviewed Jun 10, 2021

View reviewed changes

HarryMichal force-pushed the cmd/create/support-registry-authentication branch from 4ab6577 to b160671 Compare June 12, 2021 16:56

HarryMichal force-pushed the cmd/create/support-registry-authentication branch from b160671 to b88cc96 Compare June 12, 2021 17:01

HarryMichal force-pushed the cmd/create/support-registry-authentication branch from b88cc96 to 12b766c Compare June 12, 2021 17:07

debarshiray pushed a commit to HarryMichal/toolbox that referenced this pull request Jul 4, 2021

pkg/podman: Add error for 'podman pull' failing due to no authorization

4b8b007

containers#787

debarshiray pushed a commit to HarryMichal/toolbox that referenced this pull request Jul 4, 2021

pkg/podman: Wrap 'podman login'

ef3beba

containers#787

debarshiray pushed a commit to HarryMichal/toolbox that referenced this pull request Jul 4, 2021

.zuul: Bump timeout of system tests from 20 to 25 minutes

f160b2e

The new additions to the system tests makes them take longer to run, and was causing them to timeout. containers#787

debarshiray force-pushed the cmd/create/support-registry-authentication branch from ce01f2a to f160b2e Compare July 4, 2021 22:30

debarshiray requested changes Jul 4, 2021

View reviewed changes

debarshiray changed the title ~~cmd/create, pkg/podman: Add capabilities for logging to registries~~ Support logging into a registry if necessary Jul 4, 2021

debarshiray mentioned this pull request Jul 4, 2021

RFE: Migration path for users of coreos/toolbox #689

Closed

HarryMichal and others added 9 commits July 5, 2021 01:12

pkg/podman: Try to parse the error from 'podman pull' into a Go error

ecd8927

containers#689 containers#787

pkg/podman: Add error for 'podman pull' failing due to no authorization

5c7904f

containers#689 containers#787

pkg/podman: Wrap 'podman login'

674ce39

containers#689 containers#787

cmd/create: Split out the spinner around 'podman pull'

b39cb80

A subsequent commit will use this when retrying the 'podman pull' after logging the user into the registry for images that require authorization. containers#689 containers#787

.zuul: Bump timeout of system tests from 20 to 25 minutes

ba2fc50

The new additions to the system tests makes them take longer to run, and was causing them to timeout. containers#689 containers#787

debarshiray force-pushed the cmd/create/support-registry-authentication branch from f160b2e to ba2fc50 Compare July 4, 2021 23:13

This was referenced Jul 4, 2021

Would be nice to have a different 'podman pull' exit code for authorization failures containers/podman#10858

Closed

Registry authentication is not supported #754

Closed

HarryMichal mentioned this pull request Jul 7, 2021

cmd: Add function to determine if a spinner can be started #826

Closed

HarryMichal closed this Jul 15, 2021

HarryMichal mentioned this pull request Nov 30, 2021

cmd/create: Add option --authfile #935

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support logging into a registry if necessary #787

Support logging into a registry if necessary #787

HarryMichal commented Jun 7, 2021 •

edited

Loading

softwarefactory-project-zuul bot commented Jun 7, 2021

softwarefactory-project-zuul bot commented Jun 8, 2021

softwarefactory-project-zuul bot commented Jun 8, 2021

HarryMichal commented Jun 8, 2021

softwarefactory-project-zuul bot commented Jun 8, 2021

softwarefactory-project-zuul bot commented Jun 9, 2021

HarryMichal commented Jun 9, 2021 •

edited

Loading

tpopela Jun 10, 2021

HarryMichal Jun 12, 2021

softwarefactory-project-zuul bot commented Jul 4, 2021

debarshiray left a comment

debarshiray Jul 4, 2021

debarshiray Jul 4, 2021

softwarefactory-project-zuul bot commented Jul 4, 2021

softwarefactory-project-zuul bot commented Jul 4, 2021

debarshiray commented Jul 5, 2021

HarryMichal commented Jul 15, 2021

Support logging into a registry if necessary #787

Support logging into a registry if necessary #787

Conversation

HarryMichal commented Jun 7, 2021 • edited Loading

softwarefactory-project-zuul bot commented Jun 7, 2021

softwarefactory-project-zuul bot commented Jun 8, 2021

softwarefactory-project-zuul bot commented Jun 8, 2021

HarryMichal commented Jun 8, 2021

softwarefactory-project-zuul bot commented Jun 8, 2021

softwarefactory-project-zuul bot commented Jun 9, 2021

HarryMichal commented Jun 9, 2021 • edited Loading

tpopela Jun 10, 2021

Choose a reason for hiding this comment

HarryMichal Jun 12, 2021

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Jul 4, 2021

debarshiray left a comment

Choose a reason for hiding this comment

debarshiray Jul 4, 2021

Choose a reason for hiding this comment

debarshiray Jul 4, 2021

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Jul 4, 2021

softwarefactory-project-zuul bot commented Jul 4, 2021

debarshiray commented Jul 5, 2021

HarryMichal commented Jul 15, 2021

HarryMichal commented Jun 7, 2021 •

edited

Loading

HarryMichal commented Jun 9, 2021 •

edited

Loading